Skip to content

Conversation

baywet
Copy link
Member

@baywet baywet commented Feb 4, 2025

makes document for references optional. Cleans up internal constructors for references, makes references immutable to avoid side effects

related #1998

Signed-off-by: Vincent Biret <[email protected]>
Signed-off-by: Vincent Biret <[email protected]>
Signed-off-by: Vincent Biret <[email protected]>
Copy link

sonarqubecloud bot commented Feb 4, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
70.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@baywet
Copy link
Member Author

baywet commented Feb 4, 2025

@captainsafia this is where I wanted to get:

  • The proxy design pattern is properly implemented, one interface, two implementations. The consumers can use type assertions while reading a document to know what's mutable, what's not.
  • All reference types now offer a single public constructor, which only requires the reference ID but also accepts document, external resource. This way creating a document from the DOM remains simple/doesn't require carrying document references all over the place.
  • The target property is a pure function (no more backing field with potential side effects)
  • The reference property is immutable (no more side effects)
  • The properties of the reference type are also immutable (no more side effects)

The only drawback I can see is having to call RegisterComponents and/or SetReferenceHostDocument on the OpenAPIDocument based on what the consumer is doing. But we could "hide that" by calling those methods before serialization.

We'll try to release soon, in fact #2115 is already up so you can test that ASAP. I've noticed a much better developer experience and reliability on my end in kiota, openapi.net.odata, plugins, and API manifest libs. (linking locally)

@baywet baywet merged commit 878dd08 into dev Feb 5, 2025
13 of 14 checks passed
@baywet baywet deleted the fix/immutable-reference branch February 5, 2025 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants